Skip to content

Conversation

@corneadoug
Copy link
Contributor

Fixing MultiEvent Listener in Zeppelin

There was an annoying bug in zeppelin:

After moving from one note to another, Running the whole note, or adding new paragraph in a note would be done X times.

There is a simple reason related to angularJs: We have overused $rootScope.$on.

When a $rootScope.$on (Event Listener) is registered, it is registered with the $rootScope. Which means that even if the Controller was flushed after a changing page, the $rootScope is not destroy and the event listener is still register for the old Controller.

How to Use Events in AngularJS/Zeppelin

  • $rootScope.$on event listeners should only be used in MainCtrl which is the top of the App.
  • Use $scope.$on for every other controllers to listen to Events
  • Use $scope.$emit to send an event to a parent Controller (for example: paragraphCtrl to notebookCtrl)
  • Use $scope.$broadcast to send an event to a child Controller (for example: notebookCtrl to paragraphCtrl)
  • Use $rootScope.$emit to send an event to a controller that is not your parent (for example: paragraphCtrl to navCtrl), this event should be catch by the MainCtrl with a $rootScope.$on, then sent back to the rightful receiver using $scope.$broadcast.

Current ControllerStructure

MainCtrl: {
    NavCtrl,
    NotebookCtrl: [ paragraphCtrl, ...]
}

@corneadoug
Copy link
Contributor Author

Ready to Merge, I checked all events and made sure they were using the right $scope or $rootScope

@swkimme
Copy link
Contributor

swkimme commented Feb 6, 2015

Oh, man it was really annoying. Thanks for fix it!

@corneadoug
Copy link
Contributor Author

I guess it fix the #206

@Leemoonsoo
Copy link
Contributor

Awesome!

@Leemoonsoo
Copy link
Contributor

I tested this PR and it fixes the multi event catch problem.
However this branch have trouble with updating output and progress of paragraph.

@corneadoug
Copy link
Contributor Author

Can you give an example so that I can reproduce? I don't really understand what you mean by updating output and progress. I tried in multi browser and it was working well

@Leemoonsoo
Copy link
Contributor

@corneadoug Can you run any paragraph and see is there any output and progress information updated?

@corneadoug
Copy link
Contributor Author

@Leemoonsoo you were right, one change was missing. It is fixed now.

@Leemoonsoo
Copy link
Contributor

Tested and looks good to me! Cool!

@anthonycorbacho
Copy link
Contributor

Nice improvement!

corneadoug added a commit that referenced this pull request Feb 9, 2015
@corneadoug corneadoug merged commit 078e77c into master Feb 9, 2015
@corneadoug corneadoug deleted the fix/MultiRun-MultiNewParagraph branch February 9, 2015 06:04
swkimme added a commit that referenced this pull request Feb 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants